-
Notifications
You must be signed in to change notification settings - Fork 594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
interfaces/builtin/opengl: enable parsing of nvidia driver information files #14893
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #14893 +/- ##
=========================================
Coverage ? 78.27%
=========================================
Files ? 1158
Lines ? 153471
Branches ? 0
=========================================
Hits ? 120122
Misses ? 25944
Partials ? 7405
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
interfaces/builtin/opengl.go
Outdated
@@ -103,6 +103,8 @@ const openglConnectedPlugAppArmor = ` | |||
# nvidia | |||
/etc/vdpau_wrapper.cfg r, | |||
@{PROC}/driver/nvidia/params r, | |||
@{PROC}/driver/nvidia/gpus/[0-9]*:[0-9]*:[0-9]*.[0-9]*/information r, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the 'bus location' known to be only pci-like, or would it make sense to use more generic pattern here?
@{PROC}/driver/nvidia/gpus/[0-9]*:[0-9]*:[0-9]*.[0-9]*/information r, | |
@{PROC}/driver/nvidia/gpus/*/information r, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly I'm not sure.
I was going for the safest default, but I would be in favour of switching to a more generic pattern too for simplicity/compatibility if there is no objection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated to the wildcard. I can revert if anyone objects.
@@ -103,6 +103,8 @@ const openglConnectedPlugAppArmor = ` | |||
# nvidia | |||
/etc/vdpau_wrapper.cfg r, | |||
@{PROC}/driver/nvidia/params r, | |||
@{PROC}/driver/nvidia/gpus/[0-9]*:[0-9]*:[0-9]*.[0-9]*/information r, | |||
@{PROC}/driver/nvidia/capabilities/mig/monitor r, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw, what other files are present there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On my system I see:
[zyga@ciri ~]$ find /proc/driver/nvidia/ -ls
4026532876 0 dr-xr-xr-x 11 root root 0 sty 8 08:14 /proc/driver/nvidia/
4026532887 0 dr-xr-xr-x 3 root root 0 sty 8 08:14 /proc/driver/nvidia/gpus
4026532904 0 dr-xr-xr-x 5 root root 0 sty 8 08:14 /proc/driver/nvidia/gpus/0000:02:00.0
4026532907 0 -r--r--r-- 1 root root 0 sty 8 11:07 /proc/driver/nvidia/gpus/0000:02:00.0/power
4026532906 0 -rw-r--r-- 1 root root 0 sty 8 11:07 /proc/driver/nvidia/gpus/0000:02:00.0/registry
4026532905 0 -r--r--r-- 1 root root 0 sty 8 11:07 /proc/driver/nvidia/gpus/0000:02:00.0/information
4026532878 0 -r--r--r-- 1 root root 0 sty 8 08:14 /proc/driver/nvidia/params
4026532884 0 dr-xr-xr-x 3 root root 0 sty 8 11:07 /proc/driver/nvidia/patches
4026532885 0 -r--r--r-- 1 root root 0 sty 8 11:07 /proc/driver/nvidia/patches/README
4026532881 0 -rw-r--r-- 1 root root 0 sty 8 08:36 /proc/driver/nvidia/suspend
4026532886 0 -r--r--r-- 1 root root 0 sty 8 11:07 /proc/driver/nvidia/version
4026532879 0 -rw-r--r-- 1 root root 0 sty 8 11:07 /proc/driver/nvidia/registry
4026532882 0 dr-xr-xr-x 3 root root 0 sty 8 11:07 /proc/driver/nvidia/warnings
4026532883 0 -r--r--r-- 1 root root 0 sty 8 11:07 /proc/driver/nvidia/warnings/README
4026532888 0 dr-xr-xr-x 5 root root 0 sty 8 08:15 /proc/driver/nvidia/capabilities
4026532900 0 dr-xr-xr-x 4 root root 0 sty 8 08:15 /proc/driver/nvidia/capabilities/mig
4026532901 0 -r--r--r-- 1 root root 0 sty 8 08:15 /proc/driver/nvidia/capabilities/mig/config
4026532902 0 -r--r--r-- 1 root root 0 sty 8 08:15 /proc/driver/nvidia/capabilities/mig/monitor
4026533128 0 dr-xr-xr-x 3 root root 0 sty 8 11:07 /proc/driver/nvidia/capabilities/gpu0
4026533129 0 dr-xr-xr-x 2 root root 0 sty 8 11:07 /proc/driver/nvidia/capabilities/gpu0/mig
4026532903 0 -r--r--r-- 1 root root 0 sty 8 11:07 /proc/driver/nvidia/capabilities/fabric-imex-mgmt
4026532880 0 -rw-r--r-- 1 root root 0 sty 8 11:07 /proc/driver/nvidia/suspend_depth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[zyga@ciri ~]$ cat /proc/driver/nvidia/patches/README
The NVIDIA graphics driver's kernel interface files can be
patched to improve compatibility with new Linux kernels or to
fix bugs in these files. When applied, each official patch
provides a short text file with a short description of itself
in this directory.
[zyga@ciri ~]$ cat /proc/driver/nvidia/warnings/README
The NVIDIA graphics driver tries to detect potential problems
with the host system and warns about them using the system's
logging mechanisms. Important warning message are also logged
to dedicated text files in this directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite know what the mig/monitor
file is for. Looking on my system I see:
[zyga@ciri ~]$ cat /proc/driver/nvidia/capabilities/mig/monitor
DeviceFileMinor: 2
DeviceFileMode: 292
DeviceFileModify: 1
The sister capabilities/mig/config
file, which is not requested by this pull request, says:
[zyga@ciri ~]$ cat /proc/driver/nvidia/capabilities/mig/config
DeviceFileMinor: 1
DeviceFileMode: 256
DeviceFileModify: 1
I think it's probably safe to allow both, but I'm also happy to +1 this as-is.
My primary goal here it to enable nvidia-persistenced to run, so I've simply added the bare minimum in order to achieve that. I'm not not overly familiar with MIG [ Multi-Instance GPU ], or how to configure it, but there is some info here about the capabilities files: https://docs.nvidia.com/datacenter/tesla/mig-user-guide/index.html#dev-based-nvidia-capabilities It seems clear from the doc that to make use of MIG, granting read access to the capabilities files is not enough by itself, although that is not what I'm trying to do here. If a use case for MIG comes up here in the future, I may try and enhance the access further at that time, and presumably at that point I would be in position to test it properly too [ which currently I'm not ]. |
f72b80c
to
fe7531c
Compare
I'm familiar with MIG although I do not have the hardware at home. Thanks for the update, LGTM |
Hi @bboozzoo - looks like this needs two approvals. Are you able to approve as well so this can be merged ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@jocado can you rebase on top of current master and force push? |
Some nvidia hardware companion utilities required access to driver information in order to operate properly, for example and nvidia-persistenced.
fe7531c
to
cb807c1
Compare
Hi @bboozzoo - of course. Done! |
Unrelated spread failure:
|
…to snap-integrity-remove-pack * sespiros/snap-integrity-remove-pack: (86 commits) multiple: remove dm-verity support from snap pack asserts: snap integrity assertion (canonical#14870) overlord: cleanup some old edges i/builtin: make auditd-support grant seccomp setpriority (canonical#14940) tests: use quotation marks to support directories with spaces (canonical#14948) t/m/snap-service-install-mode: fix line being longer than expected interfaces/opengl: Enable parsing of nvidia driver information files (canonical#14893) i/b/fwupd: allow access to dell bios recovery (canonical#14920) tests: divide spread into fundamental/non-fundamental (canonical#14785) c/snap-bootstrap: refactor systemd-mount dm-verity/overlayfs options API (canonical#14790) o/snapstate: do not restart again for snapd along the undo path inside undoUnlinkCurrentSnap (canonical#14917) release: 2.67.1 tests: fix missing spread failures in PR comments (canonical#14931) i/prompting{,requestrules}: merge rules which have identical lifespans (canonical#14757) tests: skip apparmor-prompting-integration-tests in armhf (canonical#14919) cmd/snap-bootstrap: mount drivers tree if present (canonical#14522) i/p/patterns: disallow /./ and /../ in path patterns (canonical#14774) osutil/user: look up getent executable in known host directories (canonical#14792) overlord: wait for snapd restart after requesting by undo of 'link-snap' (canonical#14850) interfaces: update template with new syscalls (canonical#14861) ...
interfaces/opengl: Enable parsing of nvidia driver information files
Some nvidia hardware companion utilities required access to driver information in order to operate properly, for example nvidia-persistenced. This enables read only access to those files.
Example file content:
/proc/driver/nvidia/gpus/0000\:70\:00.0/information
/proc/driver/nvidia/capabilities/mig/monitor